-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New setup #109
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say it's ok to remove all the files in old
since we are targeting this at a 4.x release, maybe with at least an alpha to test early on
README.md
Outdated
### Volto 18 and later | ||
|
||
Add `volto-form-block` to your add-on `package.json`: | ||
|
||
```json | ||
"dependencies": { | ||
"volto-form-block": "*" | ||
} | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify: with volto 18 we still need to add the add-on name to the addons
array and to run pnpm add volto-form-block
or pnpm install
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the add-on needs to be declared too as usual, the recommended way is to do that in your policy package (inside packages
folder.
volto.config.js must have an entry point though, normally pointing to the inner package in packages
.
Makefile
Outdated
.PHONY: backend-docker-start | ||
backend-docker-start: ## Starts a Docker-based backend for development | ||
@echo "$(GREEN)==> Start Docker-based Plone Backend$(RESET)" | ||
docker run -it --rm --name=backend -p 8080:8080 -e SITE=Plone $(DOCKER_IMAGE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs the backend add-on. iirc with -e ADDONS="collective.volto.formsupport"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pnicolli let's move the addon in here too, so we have the complete add-on parts in the same repo.
Co-authored-by: Piero Nicolli <pnicolli@users.noreply.github.com>
@sneridagh I like the frontend/backend "monorepo" idea for volto addons, I had also written some time ago at https://community.plone.org/t/monorepo-for-plone-volto-addons/18688 to seek opinions on it Some thoughts (I know you are probably in the middle of the work): |
@mamico trying for the first time here.
Unified testing e2e, unified compatible releases. Follow semantic versioning in both. Since this will be the first one, we need to polish the idea, though:
With the version 4 you mean, when we are ready, we will issue major versions? Yeah, we must, I'd say, if they are really breaking. We will need to deprecate the old repo too and point to here. |
LICENSE
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably also have a license file in the repo root, or even just a single one. I checked a couple repos and they use the two approaches (everywhere / only root):
https://github.com/adobe/react-spectrum
https://github.com/vercel/next.js
version.txt
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for? Just curious, I didn't go deep into cookieplone yet
This is another lighter approach, which I like, https://github.com/wagtail/wagtail they put mostly in the root of the repo (both setup.py and package.json). |
@ericof thanks a lot!!!! |
@mamico @cekk @pnicolli this is ready now. We've implemented a new approach in the backend, different from the plone/meta approach, which only works in backend-only Python packages. The idea is to push this approach instead of the plone/meta one. This might need time, but we need a first implementation and experimentation with it. So this way will make it to cookieplone and the new generators. Also, this will be the first full-fledged hybrid add-on so it will allow us to have lots of feedback for making the above a reality. TBH, I'd merge it asap, and deprecate the old backend repo, if possible. What do you think? Let's talk about it during today's meeting. |
@sneridagh Thanks for the awesome work, I will check it out ASAP, definitely before the end of this week. |
@mamico @pnicolli I think the things that we should settle still are:
What do you think? My take would be to unify the README, but keep the CHANGELOG separated, and do the same for the version, so each add-on has their own life. This will force us to maintain a table with the compatibilities, but as long as we stick to semantic versioning in both sides, it shouldn't be hard. |
I see two options: the first is to have a single version for backend and frontend: the second is to be very strict with semanticversion: given independent versions X.Y.Z. But any X.Y version of the frontend must work with any X.Y version of the backend and vice versa, (e.g. 4.1.9 of the frontend must work with 4.1.2 of the backend). Thus a breaking change that requires simultaneous backend and frontend changes must require at least a Y increment. I don't have a very strong opinion on the choice ... but on the balance of pros/cons at the moment the former would win IMO the CHANGELOG follows the version policy. |
@sneridagh @pnicolli there is a problemi to discuss: zc.buildout In my tests adding:
buildout does not use the package, probably due to lack of setup.py (buildout/buildout#645) I have not tested with the product egg/wheel |
I promise that once you try the new setup, it's no turning back, and everything else seems so wrong and outdated.
Give it a try!
@pnicolli maybe it's better to merge first the PRs that you told me you have ongoing. Up to you. However, I saw that we have no tests whatsoever, we can fix this too. There are a couple of concerning ESlint failures that I commented out for now, but it would be great to fix it.